Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from rusha to native (node:crypto) sha1 implementation #329

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kravetsone
Copy link

@kravetsone kravetsone commented Dec 14, 2024

Problem

Useless rusha usage

Changes

I switch from rusha to native (node:crypto) sha1 implementation

Benchmarks

Node.js

$ node benchmarks/rusha-vs-native.mjs
clk: ~3.99 GHz
cpu: AMD Ryzen 7 7700 8-Core Processor
runtime: node 22.11.0 (x64-linux)

benchmark                   avg (min … max) p75   p99    (min … top 1%)
------------------------------------------- -------------------------------
_hash with rusha              12.10 µs/iter   7.25 µs █
                        (4.66 µs … 1.27 ms) 116.62 µs █▄▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁
_hash with native            547.04 ns/iter 435.45 ns █
                      (370.08 ns … 3.61 µs)   3.58 µs █▃▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁

summary
  _hash with native
   22.12x faster than _hash with rusha

Bun

bun benchmarks/rusha-vs-native.mjs 
clk: ~5.04 GHz
cpu: AMD Ryzen 7 7700 8-Core Processor
runtime: bun 1.1.37 (x64-linux)

benchmark                   avg (min … max) p75   p99    (min … top 1%)
------------------------------------------- -------------------------------
_hash with rusha              10.00 µs/iter   4.96 µs  █
                        (2.60 µs … 2.78 ms)  45.85 µs ▂█▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁
_hash with native            471.82 ns/iter 420.00 ns █
                    (370.00 ns … 949.79 µs)   1.99 µs █▆▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁

summary
  _hash with native
   21.19x faster than _hash with rusha

Release info Sub-libraries affected

Bump level

  • Major
  • Minor
  • Patch

Libraries affected

  • All of them
  • posthog-web
  • posthog-node
  • posthog-react-native

Changelog notes

  • Swtich from pure javascript (rusha) to native (node:crypto) sha1 implementation

@marandaneto marandaneto requested review from a team and dmarticus and removed request for a team December 17, 2024 14:06
@marandaneto
Copy link
Member

@dmarticus this is used only for feature flags so your call here.

@kravetsone
Copy link
Author

This package don't need to use pure js implementation of this because it target nodejs with native implement which more faster, more secure and etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants